-
Notifications
You must be signed in to change notification settings - Fork 7
Deal with the fact that Xtensor moved all of its header files. #174
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Thanks @bangerth for tackling this. I agree. Like others I don't really understand why this change in xtensor has been made abruptly without much notice. I'd be fine to bump the minimum required version of xtensor and xtensor-python here in fastscapelib, instead of having the burden of supporting xtensor versions both before and after 0.26. Fastscapelib users are mostly using it from within Python, for which xtensor is not a runtime dependency (I can easily update the Python wheels and conda packages). However, please let me know if on the ASPECT side you'd need to support multiple xtensor and/or fastscapelib versions. |
|
Looking at the failing tests, I'm afraid that we'll need much more I think I'd prefer to bump the minimum required version of xtensor to 0.26 here. Would you mind that? For ASPECT I think that it'll be safer to pin the version of fastscapelib and xtensor anyway. This is what we do for the Fastscapelib package on conda-forge: https://github.com/conda-forge/fastscapelib-feedstock/blob/95a7483c739b26329b7105e5e78037217ef13b55/recipe/meta.yaml#L31. For xtensor it is likely that next versions will introduce other breaking changes. Likewise in Fastscapelib, which is still in early development, although here we definitely want to avoid too much disruption. |
|
I must admit that I'm confused by the error messages. When the compiler refers to, for example, |
|
The tests that fail to compile seem to have a similar issue: I didn't touch them, but they now no longer find Xtensor -- perhaps because they look for files in the 0.25 directories, but now they are in the 0.26 locations? |
|
The tests are using xtensor 0.26, installed using the conda (micromamba) package manager. You can check that, e.g., there: https://github.com/fastscape-lem/fastscapelib/actions/runs/15620883204/job/44032444103?pr=174#step:4:144.
In #176 I pinned xtensor to 0.25, which is the version now installed for CI workflows run in the main branch and all subsequent PRs. I'll add a test in CI installing the |
|
I think I'm confused how this worked before. If you have 0.26 installed, how did the tests run before given that Fastscape couldn't compile against 0.26? |
They run successfully in CI before because the installed version of xtensor was the (unpinned) last available one at that time and 0.26 wasn't released yet. Since #176 the installed version of xtensor for CI tests is explicitly set to Since #180 there's an additional CI test that installs xtensor's |
|
Oh, I see now. I had not realized that your CI had not been updated since 0.26 came out. Anyway, I'm not entirely sure how to proceed here. Can I leave this to you as a starting point for actually making it work? |
|
Yes I can take over on what you've done here, thanks! |
Xtensor has moved all of its header files into new locations -- see xtensor-stack/xtensor#2829. This is of course a colossal hassle, but it can be worked around in a way so that it should work with both the old and new header locations.
This patch seems to work for me, and I hope it also still works for the ones of you who still use Xtensor 0.25.0.